Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Fullname Feature to User Profiles #2575

Conversation

annapurna-gupta
Copy link
Contributor

##Fixes #1282

Copy link
Member

@ReimarBauer ReimarBauer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, thanks for working on this!

There are some flake8 and linter failures found by the tests

Also you added some files which we don't need in the repository
remove

  • .coverage
  • dist/mss-9.2.0-py3.11.egg
  • instance/mscolab.db
  • output_file.py
  • pytest.log

There are multiple times added ui_add_user.py, ui_mscolab_connect_dialog.py, ui_mscolab_profile_dialog.py

I have to do a closer look on the other changes,

user_record.nickname = nickname # Update nickname

# Commit changes to the database
db.session.commit()
Copy link
Member

@ReimarBauer ReimarBauer Nov 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all these db commands needs to be moved to the file_manager

likly you can use modify_user
https://github.com/Open-MSS/MSS/blob/develop/mslib/mscolab/file_manager.py#L222

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I need to move all the additional commands to file_manager, and I should revert this part to how it was before.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not mandatory. check if you can use file_manager.modify_user you can maybe reuse the existing method

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okk


except Exception as e:
# Log the error message (use logging instead of print for production)
print(f"Error updating user info: {str(e)}") # Replace with proper logging if needed
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when you need output, you have to use logging.debug, a print can create a traceback on a webserver

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okk

@@ -966,6 +966,34 @@ def delete_account(self, _=None):
if r.status_code == 200 and json.loads(r.text)["success"] is True:
self.logout()

def editfull_name(self):
if verify_user_token(self.mscolab_server_url, self.token):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is a newer syntax for this by a decorator. Have a look on the other lines where that is used

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okkkk

self.emailid.setPlaceholderText(_translate("addUserDialog", "[email protected]"))
self.passwordLabel.setText(_translate("addUserDialog", "Password:"))
self.password.setPlaceholderText(_translate("addUserDialog", "Your password"))
self.confirmPasswordLabel.setText(_translate("addUserDialog", "Confirm Password:"))
self.rePassword.setPlaceholderText(_translate("addUserDialog", "Confirm your password"))
self.emailIDLabel.setText(_translate("addUserDialog", "Email:"))
self.usernameLabel.setText(_translate("addUserDialog", "Username:"))

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this section is not needed, likly this comes from an option of the pyuic5 command.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you mean all the changes in ui_add_user_dialog.py is not needed??

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add only the fullname

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok



from . import resources_rc
if __name__ == "__main__":
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is also not needed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okkk

ui_add_user.py Outdated
@@ -0,0 +1,10 @@
# -*- coding: utf-8 -*-
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

file on wrong place?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i removed it.

@@ -0,0 +1,10 @@
# -*- coding: utf-8 -*-
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

file on wrong place?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i removed it

@@ -0,0 +1,10 @@
# -*- coding: utf-8 -*-
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

file on wrong place?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this also.

.coverage Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't need this in the repository

Copy link
Contributor Author

@annapurna-gupta annapurna-gupta Nov 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okk i have removed it..

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't need this in the repository

Copy link
Contributor Author

@annapurna-gupta annapurna-gupta Nov 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okk i have removed it..

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't need this in the repository

Copy link
Contributor Author

@annapurna-gupta annapurna-gupta Nov 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okk i have removed it..

@ReimarBauer ReimarBauer requested a review from matrss November 25, 2024 09:30
@ReimarBauer
Copy link
Member

ReimarBauer commented Nov 26, 2024

I think we should keep creating an useraccount as it is now, and add into the profile dialog the feature to set a fullname and a nickname.

We need to be able to edit it from here anyway.

image

@matrss
Copy link
Collaborator

matrss commented Nov 26, 2024

I won't be able to fully review this until next week, but just a quick comment: we already have a username field, and I understand usernames and nicknames as being the same thing. So I don't see a reason to have two nickname fields when we could just make the username changeable (if it isn't already).

Copy link
Member

@ReimarBauer ReimarBauer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comments

self.deleteAccountBtn.setText(_translate("ProfileWindow", "Delete Account"))
self.uploadImageBtn.setText(_translate("ProfileWindow", "Change Avatar"))
import resources_rc
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where does this comes from?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

likly you need to convert the ui file with
pyuic5 --from-imports ui_mscolab_profile_dialog.ui > ../qt5/ui_mscolab_profile_dialog.py

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what do you mean? i have converted .ui file to .py file .

user_record.nickname = nickname # Update nickname

# Commit changes to the database
db.session.commit()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not mandatory. check if you can use file_manager.modify_user you can maybe reuse the existing method

mslib/mscolab/models.py Show resolved Hide resolved
self.emailid.setPlaceholderText(_translate("addUserDialog", "[email protected]"))
self.passwordLabel.setText(_translate("addUserDialog", "Password:"))
self.password.setPlaceholderText(_translate("addUserDialog", "Your password"))
self.confirmPasswordLabel.setText(_translate("addUserDialog", "Confirm Password:"))
self.rePassword.setPlaceholderText(_translate("addUserDialog", "Confirm your password"))
self.emailIDLabel.setText(_translate("addUserDialog", "Email:"))
self.usernameLabel.setText(_translate("addUserDialog", "Username:"))

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add only the fullname

@@ -247,3 +263,13 @@ def retranslateUi(self, MSColabConnectDialog):
self.idpAuthTokenSubmitBtn.setText(_translate("MSColabConnectDialog", "Submit"))
self.idpAuthTopicLabel.setText(_translate("MSColabConnectDialog", "Identity Provider Authentication"))
self.statusLabel.setText(_translate("MSColabConnectDialog", "Status:"))


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the __name__ section gets in by some conversion option, we don't need that

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okk

mslib/index.py Outdated
@@ -96,10 +95,6 @@ def mss_theme(filename):

APP.jinja_env.globals.update(get_topmenu=get_topmenu)

@APP.route("/index")
Copy link
Member

@ReimarBauer ReimarBauer Dec 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

have you moved this? or why is it deleted?

http://localhost:8083/ seems broken.

output_file.py Outdated
@@ -0,0 +1,10 @@
# -*- coding: utf-8 -*-
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need this?

pytest.log Outdated Show resolved Hide resolved
self.ui,
self.ui.tr("Edit Full Name"),
self.ui.tr(
f"You're about to change the full name - '{self.active_operation_name}' "
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the reason to add here self.active_operation_name ? How is the users full name linked to an operation?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i am confused, I need to collect the user's full name and nickname from the UI, send the data to the server, and update the UI with the response from the server. Is this the correct approach?

Copy link
Member

@ReimarBauer ReimarBauer Dec 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Start with only adding the full name. See discussion by Matthias.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My question is why is the users fullname related to the operation name?

Screenshot_20241207_041506_GitHub.jpg

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry.. it's not directly related.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Start with only adding the full name. See discussion by Matthias.

where can i find it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.



def editnick_name(self):
pass
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is currently a stub, or?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not needed, or?

Copy link
Member

@ReimarBauer ReimarBauer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comments/questions

Copy link
Member

@ReimarBauer ReimarBauer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the migration script seems not correct

Are you sure you want to reset the database? This would delete all your data! (y/[n]):y
INFO [alembic.runtime.migration] Context impl SQLiteImpl.
INFO [alembic.runtime.migration] Will assume non-transactional DDL.
INFO [alembic.runtime.migration] Running downgrade 922e4d9c94e2 -> c171019fe3ee, To version 10.0.0
Traceback (most recent call last):
...
KeyError: 'fullname'

you get on this by

annapurna-gupta/MSS$ python mslib/mscolab/mscolab.py db --reset

please also undo your changes to index.py. This is our homepage.

you can see the crash by

annapurna-gupta/MSS$ python mslib/mswms/mswms.py
on your browser http://127.0.0.1:8081

@ReimarBauer
Copy link
Member

seems I am not allowed to propose changes to your PR, so that is as patch here

diff --git a/mslib/mscolab/migrations/versions/922e4d9c94e2_to_version_10_0_0.py b/mslib/mscolab/migrations/versions/922e4d9c94e2_to_version_10_0_0.py
index 35fd014c..21c353c2 100644
--- a/mslib/mscolab/migrations/versions/922e4d9c94e2_to_version_10_0_0.py
+++ b/mslib/mscolab/migrations/versions/922e4d9c94e2_to_version_10_0_0.py
@@ -21,8 +21,6 @@ def upgrade():
     # ### commands auto generated by Alembic - please adjust! ###
     with op.batch_alter_table('users', schema=None) as batch_op:
         batch_op.add_column(sa.Column('profile_image_path', sa.String(length=255), nullable=True))
-    
-    with op.batch_alter_table('users', schema=None) as batch_op:
         batch_op.add_column(sa.Column('fullname', sa.String(length=255), nullable=True))
 
     # ### end Alembic commands ###
@@ -32,8 +30,6 @@ def downgrade():
     # ### commands auto generated by Alembic - please adjust! ###
     with op.batch_alter_table('users', schema=None) as batch_op:
         batch_op.drop_column('profile_image_path')
-
-    with op.batch_alter_table('users', schema=None) as batch_op:
         batch_op.drop_column('fullname')
 
      # ### end Alembic commands ###

@annapurna-gupta
Copy link
Contributor Author

the migration script seems not correct

Are you sure you want to reset the database? This would delete all your data! (y/[n]):y INFO [alembic.runtime.migration] Context impl SQLiteImpl. INFO [alembic.runtime.migration] Will assume non-transactional DDL. INFO [alembic.runtime.migration] Running downgrade 922e4d9c94e2 -> c171019fe3ee, To version 10.0.0 Traceback (most recent call last): ... KeyError: 'fullname'

you get on this by

annapurna-gupta/MSS$ python mslib/mscolab/mscolab.py db --reset

please also undo your changes to index.py. This is our homepage.

you can see the crash by

annapurna-gupta/MSS$ python mslib/mswms/mswms.py
on your browser http://127.0.0.1:8081

so should i change migration script by using that command? and I have undone the changes to index.py.

@annapurna-gupta
Copy link
Contributor Author

seems I am not allowed to propose changes to your PR, so that is as patch here

diff --git a/mslib/mscolab/migrations/versions/922e4d9c94e2_to_version_10_0_0.py b/mslib/mscolab/migrations/versions/922e4d9c94e2_to_version_10_0_0.py
index 35fd014c..21c353c2 100644
--- a/mslib/mscolab/migrations/versions/922e4d9c94e2_to_version_10_0_0.py
+++ b/mslib/mscolab/migrations/versions/922e4d9c94e2_to_version_10_0_0.py
@@ -21,8 +21,6 @@ def upgrade():
     # ### commands auto generated by Alembic - please adjust! ###
     with op.batch_alter_table('users', schema=None) as batch_op:
         batch_op.add_column(sa.Column('profile_image_path', sa.String(length=255), nullable=True))
-    
-    with op.batch_alter_table('users', schema=None) as batch_op:
         batch_op.add_column(sa.Column('fullname', sa.String(length=255), nullable=True))
 
     # ### end Alembic commands ###
@@ -32,8 +30,6 @@ def downgrade():
     # ### commands auto generated by Alembic - please adjust! ###
     with op.batch_alter_table('users', schema=None) as batch_op:
         batch_op.drop_column('profile_image_path')
-
-    with op.batch_alter_table('users', schema=None) as batch_op:
         batch_op.drop_column('fullname')
 
      # ### end Alembic commands ###

but the checkbox labeled 'Allow edits from maintainers' is checked.

user_record.fullname = fullname # Update full name

# Commit changes to the database
_handle_db_upgrade().session.commit()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this needs to use the file_manager

self.mscolabURLLabel.setText(_translate("ProfileWindow", "Mscolab"))
self.deleteAccountBtn.setText(_translate("ProfileWindow", "Delete Account"))
self.uploadImageBtn.setText(_translate("ProfileWindow", "Change Avatar"))


from . import resources_rc
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that is an option of pyuic5 --from-imports

@ReimarBauer ReimarBauer changed the title Add Fullname and Nickname Feature to User Profiles Add Fullname Feature to User Profiles Jan 22, 2025
@ReimarBauer
Copy link
Member

I am still not able to push to your changes/repository:
Fehler: Fehler beim Versenden einiger Referenzen nach 'github.com:annapurna-gupta/MSS.git'

please apply the patch by the patch command, patch -p1 < changes_needed.patch.txt

     -pnum  or  --strip=num
          Strip the smallest prefix containing num leading slashes from each file name found in the patch file.  A sequence of one or more adjacent slashes is counted as a single slash.   This
          controls  how file names found in the patch file are treated, in case you keep your files in a different directory than the person who sent out the patch.  For example, supposing the
          file name in the patch file was

changes_needed.patch.txt

@annapurna-gupta
Copy link
Contributor Author

patch -p1 < changes_needed.patch.txt

I tried applying the patch, but I can't find the patch file. Could you please share the file or its location so I can proceed?

@ReimarBauer
Copy link
Member

ReimarBauer commented Jan 22, 2025

It is the blue word above your Posting.
I attached it as txt file. And when you click on it you can download it.

Just tested

@annapurna-gupta
Copy link
Contributor Author

It is the blue word above your Posting. I attached it as txt file. And when you click on it you can download it.

Just tested

i found it, thank you.

@@ -616,6 +617,15 @@ def set_version_name():
return jsonify({"success": True, "message": "Successfully set version name"})


@APP.route("/edit_user_info", methods=["POST"])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where is this used?

Copy link
Member

@ReimarBauer ReimarBauer Jan 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When it is planned for

image

The function should replace the Change Avatar, by an Edit.

e-mail won't be allowed to become changed

and for changing the username you need to verify that the new one was not taken.

The new Edit should also be possible to edit the Avatar.

@annapurna-gupta annapurna-gupta closed this by deleting the head repository Jan 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

user profile options
3 participants